Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tests when not building the Python package + other minor fixes #1100

Merged
merged 6 commits into from
Sep 17, 2021

Conversation

speth
Copy link
Member

@speth speth commented Sep 16, 2021

Changes proposed in this pull request

  • Disable tests that require the mechanism conversion scripts when no Python package is being built
  • Fix installation of the Python package to the stage_dir location (fixes regression from c7a5d18)
  • Fix a compiler warning raised by Clang
  • Deprecate the unused UnknownThermoPhaseModel and UnknownKineticsModel exception classes

If applicable, fill in the issue number this pull request is fixing

Closes #1095

Checklist

  • The pull request includes a clear description of this code change
  • Commit messages have short titles and reference relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • Style & formatting of contributed code follows contributing guidelines
  • The pull request is ready for review

This allows these tests to work even when the Python package is not present.
Converting between different mechanism formats require at least
the "minimal" Python package to be built.
In the default case where the 'python_prefix' variable is '$prefix',
the Python module would get installed to the literal '$prefix'
subdirectory of the staging directory, instead of actually expanding
the '$prefix' variable.

Fixes a regression introduced in c7a5d18.
@codecov
Copy link

codecov bot commented Sep 17, 2021

Codecov Report

Merging #1100 (487e561) into main (457b1a4) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1100      +/-   ##
==========================================
- Coverage   73.49%   73.49%   -0.01%     
==========================================
  Files         364      364              
  Lines       47750    47749       -1     
==========================================
- Hits        35096    35095       -1     
  Misses      12654    12654              
Impacted Files Coverage Δ
include/cantera/kinetics/KineticsFactory.h 100.00% <ø> (ø)
include/cantera/thermo/ThermoFactory.h 100.00% <ø> (ø)
test/kinetics/rates.cpp 100.00% <ø> (ø)
test/thermo/phaseConstructors.cpp 100.00% <ø> (ø)
include/cantera/kinetics/MultiRate.h 82.60% <100.00%> (ø)
test/transport/transportFromScratch.cpp 97.93% <100.00%> (-0.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 457b1a4...487e561. Read the comment docs.

Copy link
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Copy link
Member

@bryanwweber bryanwweber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the regression!

@bryanwweber bryanwweber merged commit 37599a7 into Cantera:main Sep 17, 2021
@speth speth deleted the fix-1095 branch July 23, 2024 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Many tests fail
3 participants